- 
          
- 
                Notifications
    You must be signed in to change notification settings 
- Fork 3.2k
V2.x. Back-port PR #2334 to the v2.x branch #2581
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: v2.x
Are you sure you want to change the base?
Conversation
| Codecov Report
 
 @@            Coverage Diff             @@
##             v2.x    #2581      +/-   ##
==========================================
- Coverage   90.08%   89.98%   -0.10%     
==========================================
  Files         113      113              
  Lines        5040     5038       -2     
==========================================
- Hits         4540     4533       -7     
- Misses        500      505       +5      | 
|  | ||
| }} // namespace MiscTests | ||
|  | ||
| #if defined(CATCH_PLATFORM_WINDOWS) | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test in v3 only checks with MSVC. Why the change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I certainly didn't mean to change it.
| void FatalConditionHandler::disengage_platform() { | ||
| if (!RemoveVectoredExceptionHandler(exceptionHandlerHandle)) { | ||
| CATCH_RUNTIME_ERROR("Could not unregister vectored exception handler"); | ||
| if (SetUnhandledExceptionFilter(reinterpret_cast<LPTOP_LEVEL_EXCEPTION_FILTER>(previousTopLevelExceptionFilter)) != topLevelExceptionFilter) { | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar question: the code in v3 does not need the wild reinterpret cast. Why is it needed here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought sure I just copied the code from v3. I'll look at it no later than Monday. Thanks.
| otherwise lgtm | 
This PR back-ports the PR #2334 to the v2.x branch, enabling v2.x to be used with the Address Sanitizer on Windows.